-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: use groups handler for storage #1225
Conversation
69e6ea2
to
dfa54b9
Compare
src/containers/App/Content.tsx
Outdated
function GetCapabilities() { | ||
capabilitiesApi.useGetClusterCapabilitiesQuery(undefined); | ||
return null; | ||
function GetCapabilities({children}: {children: React.ReactNode}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now the whole interface is blocked by capabilities response
I think it would better to do it in background without impact on functionality that doesn't rely on capabilities (endpoint may timeout or throw an error)
options: AxiosOptions, | ||
getState?: GetState, | ||
) { | ||
const groupsHandlerVersion = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wouldnt it be better to create enableSomeFunctionality and hook with
return useGetFeatureVersion('/viewer/sth') > 2;
and to pass these flags to functions that depend on it
instead of mixing redux state operations with api calls by passing getState function
src/types/api/compute.ts
Outdated
v1 = 'v1', | ||
v2 = 'v2', // only this versions works with sorting | ||
} | ||
export type EVersion = 'v1' | 'v2'; // only v2 versions works with sorting |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
enum looked better imho
dfa54b9
to
d3e69bc
Compare
}: StorageRequestParams & {useGroupsHandler?: boolean}, | ||
options: AxiosOptions, | ||
) { | ||
if (useGroupsHandler && version !== 'v1') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useGroupsHandler really looks like name of react hook
was really confused why we pass hooks to api =)
Please change useGroupsHandler to something else >_<
Closes #1160
CI Results
Test Status:⚠️ FLAKY
📊 Full Report
Bundle Size: 🔺
Current: 78.87 MB | Main: 78.86 MB
Diff: +0.01 MB (0.01%)
ℹ️ CI Information